Skip to content

netem: add human-readable attrs conversion (issue #480) - #1180

Open
alacrity-aya wants to merge 1 commit into
vishvananda:mainfrom
alacrity-aya:main
Open

netem: add human-readable attrs conversion (issue #480)#1180
alacrity-aya wants to merge 1 commit into
vishvananda:mainfrom
alacrity-aya:main

Conversation

@alacrity-aya

@alacrity-aya alacrity-aya commented Apr 12, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of percentage boundary values (0% and 100%) in queue discipline conversions with proper clamping and precision.

  • New Features

    • Added ability to convert kernel-format queue discipline settings back to human-readable format with accurate percentage and time value transformations.

  • Tests

    • Added round-trip conversion tests and precision validation near boundary values.

@coderabbitai

coderabbitai Bot commented Apr 12, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 851424e8-dd66-4500-8ee4-2903fae6085f

📥 Commits

Reviewing files that changed from the base of the PR and between c822ed7 and 249010d.

📒 Files selected for processing (3)
  • qdisc.go
  • qdisc_linux.go
  • qdisc_test.go
📝 Walkthrough

Walkthrough

The changes introduce bidirectional conversion functions between kernel uint32 percentage representations and human-readable float32 percentages, update the percentage scaling formula for improved boundary handling, and add a new method to convert kernel netlink Netem attributes back to high-level NetemQdiscAttrs format. Tests validate round-trip conversions and floating-point precision near upper bounds.

Changes

Cohort / File(s) Summary
Percentage Conversion Utilities
qdisc.go		 Updated Percentage2u32 to clamp inputs and use explicit float64 scaling; added new u32ToPercentage helper for inverse conversion with special-case handling near math.MaxUint32.
Netem Attributes Transformation
qdisc_linux.go		 Added ToNetemQdiscAttrs() method on *Netem to construct high-level attributes from kernel format, applying tick2Time for time fields and u32ToPercentage for percentage-based fields.
Conversion & Precision Tests
qdisc_test.go		 Added TestNetemQdiscAttrsRoundTrip to validate bidirectional kernel↔human format conversions with float tolerance; added TestU32ToPercentagePrecisionNearMax to verify precision behavior near upper bound.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hops through percentages fine,
Converting up and down the line,
From kernel-speak to human tongue,
Round-trip conversions, neatly strung,
No precision shall be lost in sight,
The math is clamped and oh, so right! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a human-readable attributes conversion method for netem qdisc, directly addressing issue #480.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@alacrity-aya
alacrity-aya marked this pull request as ready for review April 12, 2026 02:55
@alacrity-aya

alacrity-aya commented Apr 16, 2026

Copy link
Copy Markdown
Author

Hi @aboch, could you please take a look at this PR when you have a moment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@alacrity-aya